Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reduce concurrent reconciles of the same virtual resource #2246

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lizardruss
Copy link
Contributor

@lizardruss lizardruss commented Oct 25, 2024

What issue type does this pull request address? (keep at least one, remove the others)
/kind bugfix

What does this pull request do? Which issues does it resolve? (use resolves #<issue_number> if possible)
resolves ENG-4924

Please provide a short message that should be published in the vcluster release notes
Fixed an issue where vcluster incorrectly re-created pods during garbage collection

What else do we need to know?
The previous method of passing through the syncEventType would potentially enqueue the same object 4 different ways:

  • virtual object: ns/obj
  • delete virtual object: delete#ns/obj
  • physical object: host#ns/obj
  • delete physical object: delete#host#ns/obj

Usually this would not be a concern, but the following scenario occurs frequently enough to cause a conformance test to fail:

  1. virtual object is deleted
  2. a update event for the virtual object is received that adds the deletionTimestamp
  3. a delete event for the virtual object is received
  4. these events would enqueue the object as objects with the same name in different namespaces from the controller runtime's perspective
  5. the two objects are reconciled concurrently by the controller-runtime
  6. while the Reconcile function is processing the virtual object update the virtual object is deleted, this leads to the SyncToVirtual implementation incorrectly re-creating the virtual object

This PR attempts to solve this by:

  • Reducing the amount of concurrent reconciles for the same virtual object by dropping the delete# prefix
  • Using a different method of passing the syncEventType through to Reconcile (and the various sync event types). Event types are recorded by virtual object UID, only the last event is saved, and a delete event for a UID cannot be overwritten.

Copy link

netlify bot commented Oct 25, 2024

Deploy Preview for vcluster-docs canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit 6960aa8
🔍 Latest deploy log https://app.netlify.com/sites/vcluster-docs/deploys/6720fe351a72300008d0262d

@lizardruss lizardruss changed the title fix: skip syncing resources to the virtual cluster if they were not imported reduce concurrent reconciles of the same virtual resource Oct 29, 2024
@lizardruss
Copy link
Contributor Author

Currently looking into a solution that:

  • doesn't require a different data structure that potentially leaks memory
  • doesn't require a lock inside the Reconcile function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant